Temporary Tailwind4 migration for MotiveMarket#1950
Conversation
| <span class="text-accent-50 font-bold">{{ resultsLength }}</span> | ||
| <span>of</span> | ||
| <span class="x-text-accent-50">{{ totalResults }}</span> | ||
| <span class="text-accent-50">{{ totalResults }}</span> |
There was a problem hiding this comment.
text-accent is from the x-tailwindcss should keep be with x-, right? same for x-font-bold etc etc
There was a problem hiding this comment.
Yes but not. Both classes are generated from x-tailwindcss but they are from tailwind.
font-bold: https://tailwindcss.com/docs/font-weight
text-{color}-{tone}: https://tailwindcss.com/docs/color
| color: shadeColor, | ||
| })), | ||
| { prefix: '&-' }, | ||
| return mapColorsFlat((color, colorName) => { |
There was a problem hiding this comment.
| return mapColorsFlat((color, colorName) => { | |
| return mapColors((color, colorName) => { |
| ) | ||
| } | ||
|
|
||
| export function mapColorsFlat<T extends CssStyleOptions>( |
There was a problem hiding this comment.
| export function mapColorsFlat<T extends CssStyleOptions>( | |
| export function mapColors<T extends CssStyleOptions>( |
There was a problem hiding this comment.
I would like to know why two different functions mapColors and mapColorsFlat are needed for the migration
| export interface CSSRuleObject { | ||
| [selector: string]: CSSRuleValue | string | number | undefined | ||
| } |
There was a problem hiding this comment.
| export interface CSSRuleObject { | |
| [selector: string]: CSSRuleValue | string | number | undefined | |
| } |
This should be in the type.ts right?
| @config './tailwind.config.ts'; | ||
|
|
||
| @plugin "@empathyco/x-tailwindcss/plugin"; | ||
| @plugin "@empathyco/x-tailwindcss/old-ds-plugin"; |
There was a problem hiding this comment.
We should get rid of this:
| @plugin "@empathyco/x-tailwindcss/old-ds-plugin"; |
There was a problem hiding this comment.
Could we remove this? If something is needed (Variables or whatever), move it where necessary. x-components or x-tailwind-plugin.
1c5747f to
07a1e4c
Compare
There was a problem hiding this comment.
I won't change the CHANGELOG to know what we have in the past
| "moduleResolution": "node", | ||
| "types": ["tailwindcss"], | ||
| "paths": { | ||
| "tailwindcss/plugin": ["./node_modules/tailwindcss/dist/plugin.d.mts"] |
There was a problem hiding this comment.
Can we explain this change ?
| "@rollup/plugin-commonjs": "29.0.0", | ||
| "@tailwindcss/postcss": "4.1.17", | ||
| "@tailwindcss/vite": "4.1.17", | ||
| "@types/tailwindcss": "^3.1.0", |
There was a problem hiding this comment.
| "@types/tailwindcss": "^3.1.0", | |
| "@types/tailwindcss": "3.1.0", |
| vueDocsPlugin, | ||
| Inspector(), | ||
| tailwindcss({ | ||
| config: resolve(__dirname, './tailwind.config.ts'), |
There was a problem hiding this comment.
This file was moved to src/tailwind/tailwind.config.ts right?
| @@ -0,0 +1,4 @@ | |||
| @import 'tailwindcss'; | |||
There was a problem hiding this comment.
| @import 'tailwindcss'; | |
| @import 'tailwindcss' prefix(xds); |
This tailwind demo is not only for showcase. It is also a development tool for setups. Usually, a developer runs this project, chooses a styling to apply, and copies the whole bunch of classes. There is a feature to copy styles from the demo.
With this prefix change, this feature becomes useless, as we would need to add the prefix manually again, which will be the case for 99.9999% of the cases.
| @config './tailwind.config.ts'; | ||
|
|
||
| @plugin "@empathyco/x-tailwindcss/plugin"; | ||
| @plugin "@empathyco/x-tailwindcss/old-ds-plugin"; |
There was a problem hiding this comment.
Could we remove this? If something is needed (Variables or whatever), move it where necessary. x-components or x-tailwind-plugin.
| ...badgeBright(helpers), | ||
| }, | ||
| { | ||
| prefix: '.x-badge-', |
There was a problem hiding this comment.
| prefix: '.x-badge-', | |
| prefix: '.badge-', |
Shouldn't be like that?
| }, | ||
| { | ||
| prefix: '&-', | ||
| prefix: '&.input-', |
There was a problem hiding this comment.
| prefix: '&.input-', | |
| prefix: '.input-', |
Right?
| { | ||
| ...textSizes(helpers), | ||
| }, | ||
| { prefix: '&.text1-' }, |
| { | ||
| ...textSizes(helpers), | ||
| }, | ||
| { prefix: '&.text2-' }, |
| { prefix: '&-' }, | ||
| { prefix: '.layout-' }, | ||
| ), | ||
| ...minMargin(helpers), |
There was a problem hiding this comment.
Why minMargin and maxWidth now are out of rename?
| ) | ||
| } | ||
|
|
||
| export function mapColorsFlat<T extends CssStyleOptions>( |
There was a problem hiding this comment.
I would like to know why two different functions mapColors and mapColorsFlat are needed for the migration
| }, | ||
| ) | ||
|
|
||
| const xTailwindPlugin: unknown = _xTailwindPlugin |
There was a problem hiding this comment.
Why export something unknown?
| "main": "dist/cjs/index.js", | ||
| "module": "dist/esm/index.js", | ||
| "types": "dist/types/index.d.ts", | ||
| "exports": { |
There was a problem hiding this comment.
Why does the export process now have to be done this way?
| @@ -0,0 +1,4 @@ | |||
| @import 'tailwindcss'; | |||
|
|
|||
| @plugin "../../../src/x-tailwind-plugin/plugin"; | |||
There was a problem hiding this comment.
Remains tailwind.config.ts in the demo root with plugins: [xTailwindCss],. Are both needed?
| <BaseEventButton | ||
| :events="events" | ||
| class="x-events-modal-close-button x-button" | ||
| class="x-events-modal-close-button xds:button" |
There was a problem hiding this comment.
Using both prefix x and xds is something that drives me crazy 😅
I know the reason behind but I don't like the result mix
|
|
||
| <!-- Facets --> | ||
| <Facets class="x-gap-24"> | ||
| <Facets class="gap-24"> |
There was a problem hiding this comment.
Should this be xds: prefixed?
| @@ -0,0 +1,4 @@ | |||
| @import 'tailwindcss'; | |||
There was a problem hiding this comment.
I think I would prefix the demo too to make it more realistic since supposedly it will to be the way we will use the XDS in consumers
| * @public | ||
| * The helpers provided internally by our Tailwind plugin. | ||
| * Tailwind CSS 4 no longer exposes `PluginAPI`, | ||
| * so we recreate the pieces we need. |
There was a problem hiding this comment.
This comment explains very well that we're giving Tailwind 3 a facelift to make it look like Tailwind 4. From my point of view, it's not really a migration.
# Conflicts: # packages/x-components/package.json # packages/x-tailwindcss/package.json # pnpm-lock.yaml
438933c to
dd29ddb
Compare
dd29ddb to
4fb3dbe
Compare
Pull request template
Describe the purpose of the change, the specific changes done in detail, and the issue you have fixed.
Motivation and context
Type of change
What is the destination branch of this PR?
MainHow has this been tested?
Tests performed according to testing guidelines:
Checklist: